Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linting run over once, pushing up for review. #3

Open
wants to merge 2 commits into
base: fhir-r4
Choose a base branch
from

Conversation

eawest2
Copy link

@eawest2 eawest2 commented May 19, 2020

What does this PR do?

Posting for Henry to check his codework against SONARlint

Related JIRA tickets:

SAR-47

How should this be manually tested?

Run the code through SONARlint, and run the tests.

@eawest2 eawest2 added the enhancement New feature or request label May 19, 2020
@eawest2 eawest2 requested a review from hmayenve May 19, 2020 18:55
@eawest2 eawest2 self-assigned this May 19, 2020
@hmayenve hmayenve requested a review from mhiner May 20, 2020 12:27
Copy link
Collaborator

@hmayenve hmayenve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eawest2 there are still a couple sonarlint errors that can be fixed. Errors such as Swap these 2 arguments so they are in the correct order: expected value, actual value. in ConditionTest.java Everything else looks good.

@@ -56,8 +56,8 @@ public static String generateIfNoneExist(BundleEntryComponent bundleEntry) {
// find any overriding OIDs.
String[] subsetOIDArray = getDefinedOIDs(bundleEntry.getResource().getResourceType().name());

if (bundleEntry.getResource().getResourceType().name() != "Medication"
&& bundleEntry.getResource().getResourceType().name() != "PractitionerRole") {
if (bundleEntry.getResource().getResourceType().name().equals("Medication") == false
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another way of doing this is if(!bundleEntry.getResource().getResourceType().name().equals("Medication"))

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I mostly just wanted it to be explicit what the intent of the statement was without having to add comments to the code.

@eawest2
Copy link
Author

eawest2 commented May 20, 2020

@eawest2 there are still a couple sonarlint errors that can be fixed. Errors such as Swap these 2 arguments so they are in the correct order: expected value, actual value. in ConditionTest.java Everything else looks good.

I'm not sure about those ones in ConditionTest, most of them are explicit assertions of values, so the order they're in matters, since it'll be the order they're delivered in. Those tests will fail if I reorder the variables.

@rmharrison rmharrison changed the base branch from fhir-stu3 to fhir-r4 July 1, 2020 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants